test(install): isolate Claude smoke host roots - #5290
Conversation
Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
huangruiteng
left a comment
There was a problem hiding this comment.
Approval conclusion (author-owned PR; GitHub blocks formal self-approval)
评审 head:a97c75a705dbe53209f696354d6933b9eb5d23f7。结论:APPROVE,未发现本 PR 的阻断项。按 LoopX PR review capability 完成 smoke 价值及失败归因检查;未执行合并。
动机
仅设置临时 HOME 并不足以隔离安装 smoke:继承的 CODEX_HOME 等覆盖项仍能让真实 installer 写到测试目录以外。这个 PR 修复测试本身的写入边界,防止开发者验证产品时修改自己的已安装 skills,而不是把绿色 smoke 当作真实隔离证据。
改动思路
保留实际 bash installer 和 Claude installer 子进程,改为构造明确的 fixture 环境。父进程故意注入 fixture 外的各类根目录,子进程只收到允许的环境项;检查预期 skills 确实出现在 fixture 内,同时外部目录仅保留原有哨兵。project/user scope、dry-run、hooks opt-in 和权限保留测试继续走真实脚本。没有修改生产安装器,也没有给宿主安装更多权限。
具体改动
仅修改既有 smoke,增加 44 行、删除 15 行,没有新增一次性示例或生产模块。
关键代码讲解
_isolated_host_env(examples/claude-install-optin-smoke.py:40)明确提供测试 HOME、PATH、shell 和三个宿主配置根,不再复制整个父进程环境。其职责是测试 fixture 隔离,不改变生产 installer 解释用户配置的方式。main(同文件:51)的默认安装分支为父进程设置外部根目录反例,再调用真实 install-local.sh;新增 Codex skill 的内部存在检查以及外部目录/哨兵不变检查。因此“测试没写任何东西”不能伪装成隔离成功。_install_py(同文件:36,未修改)的真实调用在 project/user 分支使用相同隔离环境。现有无 scope 拒绝、dry-run 无写入、project 不写全局目录、默认不写 hooks、harden 保留既有 deny 和用户 hook 的断言继续有效。显式 project harden 分支只写其临时 project;未扩大到全局宿主。
对主干的风险
实际 base/head 反例使用同一受控外部 Codex 根和相同未修改 smoke:基线 smoke 返回成功,却写入外部根的 14 个 skill 文件;head 同样返回成功,但外部根只剩原有哨兵,内容不变。这证明新增断言不是只重复检查成功码。基线为 ee1ea64b0aef45fdda81d2d7e48da356a1750eab,配对 fixture 指纹为 e11a812d36180feec10acb147092ce54d4422a0f1a7e8eb0cedfcf258773c48f。全部外部路径都属于临时合成 fixture,未触碰真实用户根。
修改后的原始 smoke 独立重跑通过,配对 head 的完整 smoke 也通过;相邻 no-system-mutation smoke、Ruff 和编译检查通过。首次原始运行曾在 candidate doctor 阶段失败,未获得细分原因;保留这一历史失败,不把它编造成外部故障。后续相同 head、未修改 smoke 的独立重跑及配对执行通过,独立 deep installation doctor 的 6 项 required 检查也通过;因此当前验收证据不是首次失败的成功码改写。
选中 premerge 的 4 项直接检查通过,6 项选中检查中 5 项通过。唯一剩余失败为未修改的 install-local-smoke 超时:同一命令、同一 120 秒预算在不可变 base 和本 head 均为 timed_out,returncode 为空且完整输出均为空。脚本及生产 install-local/Claude installer 的 base/head SHA-256 一致;该脚本不调用本 PR 修改的 smoke。结合上面的独立隔离反例通过,将此失败归为 pre_existing_unrelated,保留其安装验证预算排查,不增加 timeout 或宣称全部 canary 绿色。当前策略不查询或等待远端 CI。
已扫描 install-local、Claude no-system-mutation、slash installer 的既有覆盖及同作者近期安装相关 PR:它们没有覆盖此 smoke 子进程继承宿主根的漏洞;#5151 的生产安装 guard 修复、#5307 的 deprecated skills alias 清理也不是本次 fixture 隔离修复。增加断言位于原有 smoke,具备防止验证污染开发者环境的持续价值,不是重复批量 scaffolding。
我的整体评价
long_horizon:improved,反复执行安装验证不会积累测试外 skill 写入。user_experience:improved,维护者可以安全运行原有命令,scope、opt-in 和用户权限保留的产品合同不变。机制与问题相称;向前精简检查已把重复环境构造收束为局部 helper,无需提升成公共 capability。它不修改产品界面或共享控制面语义,不能据此声称整套 installer 性能验收完成。基线/head 的 120 秒超时须另行诊断,但不应让一个已经证明无关的红检查把本 PR 变成 request changes。
English verdict: APPROVE - a97c75a. Real baseline/head execution proves the smoke no longer writes inherited host roots; focused rerun, deep doctor and adjacent boundary validation passed. The unchanged installer canary timed out identically on immutable base and head and is separately attributed, not a PR regression. No merge or remote CI polling.
The Claude install smoke set a synthetic
HOMEbut inherited host-root overrides such asCODEX_HOME. A normal default-install check could therefore write slash skills into the caller's live Codex home while still passing its Claude-only assertion.The existing smoke now launches the real local installer with synthetic Codex, Claude, and OpenCode roots and no inherited LoopX install paths. It injects outside host-root overrides into the parent process, checks that the Codex and Claude
/loopxskills land inside the fixture, and verifies the outside root remains untouched. The existing adapter-off, project/user scope,--harden, and permission-preservation checks remain in the same smoke. This repairs a pre-existing fixture gap observed during #5151 qualification; installer behavior is unchanged.Validation: the real
claude-install-optin-smoke.pypassed twice, the adjacent Claude installer smoke passed, and Ruff, Python compilation, diff hygiene, and the changed-file public-boundary scan passed. The selected premerge catalog is still red:install-local-smoke.pyexceeded its existing 120-second per-check limit on both attempts; one run ofsemantic-vocabulary-drift-smoke.pylacked npm development dependencies and passed afternpm ci --ignore-scripts;codex-cli-packaged-install-smoke.pypassed once and then hit a temporary-directory cleanup race on retry. The other selected canaries passed. These catalog checks do not import the changed smoke. Keep this draft unmerged until independent review and CI resolve the remaining validation gap.